libostree: Add multiple getter/setter support to OstreeAsyncProgress

OstreeAsyncProgress is thread-safe: it can have keys changed by one
thread while another is getting the same keys (modulo some locking
contention). However, the thread safety is done at the function call
level: if some code calls an OstreeAsyncProgress getter several times,
the key fetches are not atomic with respect to each other.

In the case of contention on the lock, this can result in consumers of
OstreeAsyncProgress data seeing an inconsistent state between the
properties they query, which could result in progress reporting
inaccuracies.

In the uncontested case, this results in the OstreeAsyncProgress lock
being locked and unlocked many times more than necessary.

Try to improve this by adding new API, which supports getting and
setting multiple keys atomically:
 • ostree_async_progress_get()
 • ostree_async_progress_set()

The new API uses GVariants and varargs: keys are passed as a
GVariantType string followed by arguments as for g_variant_new() or
g_variant_get(), followed by the next key, etc.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #819
Approved by: cgwalters

diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt
index 5625a1936d740c1712498e63e148f59badf1c0b9..adc2dfd74078ac7279da571c8be81ad7d2145cd7 100644 (file)
--- a/apidoc/ostree-sections.txt
+++ b/apidoc/ostree-sections.txt
@@ -4,10 +4,12 @@ OstreeAsyncProgress
 ostree_async_progress_new
 ostree_async_progress_new_and_connect
 ostree_async_progress_get_status
+ostree_async_progress_get
 ostree_async_progress_get_variant
 ostree_async_progress_get_uint
 ostree_async_progress_get_uint64
 ostree_async_progress_set_status
+ostree_async_progress_set
 ostree_async_progress_set_variant
 ostree_async_progress_set_uint
 ostree_async_progress_set_uint64

diff --git a/src/libostree/libostree.sym b/src/libostree/libostree.sym
index 0787d87722efdf6fa479aa31899cd9b35f1bf716..df74de53258d1655a1adcc3e50308706c4249b5f 100644 (file)
--- a/src/libostree/libostree.sym
+++ b/src/libostree/libostree.sym
@@ -396,6 +396,8 @@ global:
 
 LIBOSTREE_2017.6 {
 global:
+  ostree_async_progress_get;
+  ostree_async_progress_set;
   ostree_async_progress_get_variant;
   ostree_async_progress_set_variant;
 } LIBOSTREE_2017.4;

diff --git a/src/libostree/ostree-async-progress.c b/src/libostree/ostree-async-progress.c
index 68f4c8349f8a89db88ea121b8234f153fadeeaf3..4927a9dedf87fb6b41406839f1b40466c1dd6acf 100644 (file)
--- a/src/libostree/ostree-async-progress.c
+++ b/src/libostree/ostree-async-progress.c
@@ -157,6 +157,68 @@ ostree_async_progress_get_uint64 (OstreeAsyncProgress       *self,
   return (rval != NULL) ? g_variant_get_uint64 (rval) : 0;
 }
 
+/**
+ * ostree_async_progress_get:
+ * @self: an #OstreeAsyncProgress
+ * @...: key name, format string, #GVariant return locations, …, followed by %NULL
+ *
+ * Get the values corresponding to zero or more keys from the
+ * #OstreeAsyncProgress. Each key is specified in @... as the key name, followed
+ * by a #GVariant format string, followed by the necessary arguments for that
+ * format string, just as for g_variant_get(). After those arguments is the
+ * next key name. The varargs list must be %NULL-terminated.
+ *
+ * Each format string must make deep copies of its value, as the values stored
+ * in the #OstreeAsyncProgress may be freed from another thread after this
+ * function returns.
+ *
+ * This operation is thread-safe, and all the keys are queried atomically.
+ *
+ * |[<!-- language="C" -->
+ * guint32 outstanding_fetches;
+ * guint64 bytes_received;
+ * g_autofree gchar *status = NULL;
+ * g_autoptr(GVariant) refs_variant = NULL;
+ *
+ * ostree_async_progress_get (progress,
+ *                            "outstanding-fetches", "u", &outstanding_fetches,
+ *                            "bytes-received", "t", &bytes_received,
+ *                            "status", "s", &status,
+ *                            "refs", "@a{ss}", &refs_variant,
+ *                            NULL);
+ * ]|
+ *
+ * Since: 2017.6
+ */
+void
+ostree_async_progress_get (OstreeAsyncProgress *self,
+                           ...)
+{
+  va_list ap;
+  const char *key, *format_string;
+
+  g_mutex_lock (&self->lock);
+  va_start (ap, self);
+
+  for (key = va_arg (ap, const char *), format_string = va_arg (ap, const char *);
+       key != NULL;
+       key = va_arg (ap, const char *), format_string = va_arg (ap, const char *))
+    {
+      GVariant *variant;
+
+      g_assert (format_string != NULL);
+
+      variant = g_hash_table_lookup (self->values, GUINT_TO_POINTER (g_quark_from_string (key)));
+      g_assert (variant != NULL);
+      g_assert (g_variant_check_format_string (variant, format_string, TRUE));
+
+      g_variant_get_va (variant, format_string, NULL, &ap);
+    }
+
+  va_end (ap);
+  g_mutex_unlock (&self->lock);
+}
+
 static gboolean
 idle_invoke_async_progress (gpointer user_data)
 {
@@ -205,6 +267,78 @@ ostree_async_progress_get_status (OstreeAsyncProgress       *self)
   return ret;
 }
 
+/**
+ * ostree_async_progress_set:
+ * @self: an #OstreeAsyncProgress
+ * @...: key name, format string, #GVariant parameters, …, followed by %NULL
+ *
+ * Set the values for zero or more keys in the #OstreeAsyncProgress. Each key is
+ * specified in @... as the key name, followed by a #GVariant format string,
+ * followed by the necessary arguments for that format string, just as for
+ * g_variant_new(). After those arguments is the next key name. The varargs list
+ * must be %NULL-terminated.
+ *
+ * g_variant_ref_sink() will be called as appropriate on the #GVariant
+ * parameters, so they may be floating.
+ *
+ * This operation is thread-safe, and all the keys are set atomically.
+ *
+ * |[<!-- language="C" -->
+ * guint32 outstanding_fetches = 15;
+ * guint64 bytes_received = 1000;
+ *
+ * ostree_async_progress_set (progress,
+ *                            "outstanding-fetches", "u", outstanding_fetches,
+ *                            "bytes-received", "t", bytes_received,
+ *                            "status", "s", "Updated status",
+ *                            "refs", "@a{ss}", g_variant_new_parsed ("@a{ss} {}"),
+ *                            NULL);
+ * ]|
+ *
+ * Since: 2017.6
+ */
+void
+ostree_async_progress_set (OstreeAsyncProgress *self,
+                           ...)
+{
+  va_list ap;
+  const char *key, *format_string;
+  gboolean changed;
+
+  g_mutex_lock (&self->lock);
+
+  if (self->dead)
+    goto out;
+
+  va_start (ap, self);
+
+  for (key = va_arg (ap, const char *), format_string = va_arg (ap, const char *);
+       key != NULL;
+       key = va_arg (ap, const char *), format_string = va_arg (ap, const char *))
+    {
+      GVariant *orig_value;
+      g_autoptr(GVariant) new_value = NULL;
+      gpointer qkey = GUINT_TO_POINTER (g_quark_from_string (key));
+
+      new_value = g_variant_ref_sink (g_variant_new_va (format_string, NULL, &ap));
+
+      if (g_hash_table_lookup_extended (self->values, qkey, NULL, (gpointer *) &orig_value) &&
+          g_variant_equal (orig_value, new_value))
+        continue;
+
+      g_hash_table_replace (self->values, qkey, g_steal_pointer (&new_value));
+      changed = TRUE;
+    }
+
+  va_end (ap);
+
+  if (changed)
+    ensure_callback_locked (self);
+
+out:
+  g_mutex_unlock (&self->lock);
+}
+
 /**
  * ostree_async_progress_set_variant:
  * @self: an #OstreeAsyncProgress

diff --git a/src/libostree/ostree-async-progress.h b/src/libostree/ostree-async-progress.h
index 62bdcff87b30d43e28a3ea78b289ed512d24bea0..55ac591cdb634a9116f85ca36a1216febd0bf084 100644 (file)
--- a/src/libostree/ostree-async-progress.h
+++ b/src/libostree/ostree-async-progress.h
@@ -53,6 +53,10 @@ OstreeAsyncProgress *ostree_async_progress_new_and_connect (void (*changed) (Ost
 _OSTREE_PUBLIC
 char *ostree_async_progress_get_status (OstreeAsyncProgress       *self);
 
+_OSTREE_PUBLIC
+void ostree_async_progress_get (OstreeAsyncProgress *self,
+                                ...) G_GNUC_NULL_TERMINATED;
+
 _OSTREE_PUBLIC
 guint ostree_async_progress_get_uint (OstreeAsyncProgress       *self,
                                       const char                *key);
@@ -67,6 +71,10 @@ _OSTREE_PUBLIC
 void ostree_async_progress_set_status (OstreeAsyncProgress       *self,
                                        const char                *status);
 
+_OSTREE_PUBLIC
+void ostree_async_progress_set (OstreeAsyncProgress *self,
+                                ...) G_GNUC_NULL_TERMINATED;
+
 _OSTREE_PUBLIC
 void ostree_async_progress_set_uint (OstreeAsyncProgress       *self,
                                      const char                *key,